Skip to content

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Oct 10, 2025

This lifts the restriction of having to put this lexically inside an effect, now it can appear anywhere as long as it's called in the context of an effect or derived. We're leveraging the existing pending logic but instead of returning a number we're executing the function that was passed and return its result. We also have a map with all the current source values which is used for the duration of the execution of said function.

Copy link

changeset-bot bot commented Oct 10, 2025

🦋 Changeset detected

Latest commit: 61adc3d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@16926

dummdidumm and others added 3 commits October 10, 2025 17:27
If cursor was at end and new input is longer, move cursor to new end

No test because not possible to reproduce using our test setup.

Follow-up to sveltejs#14649, helps with sveltejs#16577
@Rich-Harris
Copy link
Member

If we can get this to work, that'd would be way better. Right now I'm seeing some glitchy behaviour — I suspect it's less caused by this PR than revealed by it, but either way we need to investigate.

With the other PR it's possible to get into a broken state if you spam the button enough...

Screen.Recording.2025-10-13.at.2.48.02.PM.mov

...but with this PR it seems to happen much more easily:

Screen.Recording.2025-10-13.at.2.49.45.PM.mov

github-actions bot and others added 4 commits October 13, 2025 14:52
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Updated code block syntax from Svelte to JavaScript for clarity.
@Rich-Harris
Copy link
Member

Basically as soon as you have an $effect.pending(), with or without an argument, things start going wacky if promises resolve out of order — demo. If you uncomment the second line, things get wackier still — not sure if the same bug or a different one. Things are also wacky on main. Investigating.

* fix: unset context on stale promises

When a stale promise is rejected in `async_derived`, and the promise eventually resolves, `d.resolve` will be noop and `d.promise.then(handler, ...)` will never run. That in turns means any restored context (via `(await save(..))()`) will never be unset. We have to handle this case and unset the context to prevent errors such as false-positive state mutation errors

* fix: unset context on stale promises (slightly different approach) (sveltejs#16936)

* slightly different approach to sveltejs#16935

* move unset_context call

* get rid of logs

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
@dummdidumm
Copy link
Member Author

dummdidumm commented Oct 13, 2025

I'm somewhat confident this is the same bug that caused me to add the TODO there's something else buggy comment atop the one test assertion in #16935 - that stale batches are not reliably detected and so they just run, and then we end up in mark_effects with all sorts of weird sources and things go downhill from there - so yeah, not caused, but more revealed by this.

@dummdidumm
Copy link
Member Author

I wonder if #16944 fixes this

hrueger and others added 2 commits October 14, 2025 09:56
…6943)

* fix(svg radialGradient): fr attribute missing in types

* chore: add changeset
* Version Packages

* Update packages/svelte/CHANGELOG.md

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>
@Rich-Harris
Copy link
Member

I merged that branch into this one, and it's definitely a lot more stable than it is at present — there's still some glitchiness (things can get out of sync while work is ongoing) but it always ends up in a correct state

Rich-Harris and others added 10 commits October 14, 2025 08:12
* chore: simplify `batch.apply()`

* belt and braces

* note to self
Since sveltejs#16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch.

This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on sveltejs#16935
quick follow-up to sveltejs#16944
Resetting a map entry does not change its position in the map when iterating. We need to make sure that reset makes that batch jump "to the front" for the "reject all stale batches" logic below. Edge case for which I can't come up with a test case but it _is_ a possibility.
* feat: add `createContext` utility for type-safe context

* regenerate
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@Rich-Harris Rich-Harris changed the title feat: $effect.pending(value) (runtime-first approach) feat: $state.eager(value) Oct 14, 2025
@Rich-Harris Rich-Harris merged commit d8e61f6 into sveltejs:effect-pending-value Oct 14, 2025
21 of 23 checks passed
@Rich-Harris Rich-Harris changed the title feat: $state.eager(value) feat: $state.eager(value) (runtime-first approach) Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants